Skip to content

Use Node v12.13.0 (LTS) for development #4322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 21, 2019
Merged

Use Node v12.13.0 (LTS) for development #4322

merged 10 commits into from
Nov 21, 2019

Conversation

etpinard
Copy link
Contributor

Node v12 is now LTS, so let's start using it.

Note the node-v12 docker containers provided by CircleCI ship with Chrome v78. This makes some of our jasmine tests fail.


The failing tests are:

it('should act like closest mode on ternary when cartesian is in compare mode', function(done) {
var mock = Lib.extendDeep({}, require('@mocks/ternary_fill.json'));
var gd = createGraphDiv();
mock.data.push({y: [7, 8, 9]});
mock.layout.xaxis = {domain: [0.8, 1], visible: false};
mock.layout.yaxis = {domain: [0.8, 1], visible: false};
Plotly.plot(gd, mock.data, mock.layout).then(function() {
expect(gd._fullLayout.hovermode).toBe('x');
// hover over a point when that's closest, even if you're over
// a fill, because by default we have hoveron='points+fills'
assertLabelsCorrect([237, 150], [240.0, 144],
'trace 2Component A: 0.8Component B: 0.1Component C: 0.1');
// hovers over fills
assertLabelsCorrect([237, 170], [247.7, 166], 'trace 2');
// hover on the cartesian trace in the corner
assertLabelsCorrect([363, 122], [363, 122], 'trace 38');
})
.catch(failTest)
.then(done);
});

This ⬆️ one can probably be fixed be tweaking the pixels values in the assertions.

it('can drag root-level colorbars in editable mode', function(done) {
Plotly.newPlot(gd,
[{z: [[1, 2], [3, 4]], type: 'heatmap'}],
{width: 400, height: 400},
{editable: true}
)
.then(function() {
expect(gd.data[0].colorbar).toBeUndefined();
expect(gd._fullData[0].colorbar.x).toBe(1.02);
expect(gd._fullData[0].colorbar.y).toBe(0.5);
return drag({node: getCBNode(), dpos: [-100, 100]});
})
.then(function() {
expect(gd.data[0].colorbar.x).toBeWithin(0.591, 0.01);
expect(gd.data[0].colorbar.y).toBeWithin(0.045, 0.01);
})
.catch(failTest)
.then(done);
});
it('can drag marker-level colorbars in editable mode', function(done) {
Plotly.newPlot(gd,
[{y: [1, 2, 1], marker: {color: [0, 1, 2], showscale: true}}],
{width: 400, height: 400},
{editable: true}
)
.then(function() {
expect(gd.data[0].marker.colorbar).toBeUndefined();
expect(gd._fullData[0].marker.colorbar.x).toBe(1.02);
expect(gd._fullData[0].marker.colorbar.y).toBe(0.5);
return drag({node: getCBNode(), dpos: [-100, 100]});
})
.then(function() {
expect(gd.data[0].marker.colorbar.x).toBeWithin(0.591, 0.01);
expect(gd.data[0].marker.colorbar.y).toBeWithin(0.045, 0.01);
})
.catch(failTest)
.then(done);
});
it('can drag colorbars linked to color axes in editable mode', function(done) {
Plotly.newPlot(gd,
[{z: [[1, 2], [3, 4]], type: 'heatmap', coloraxis: 'coloraxis'}],
{coloraxis: {}, width: 400, height: 400},
{editable: true}
)
.then(function() {
expect(gd.layout.coloraxis.colorbar).toBeUndefined();
expect(gd._fullLayout.coloraxis.colorbar.x).toBe(1.02);
expect(gd._fullLayout.coloraxis.colorbar.y).toBe(0.5);
return drag({node: getCBNode(), dpos: [-100, 100]});
})
.then(function() {
expect(gd.layout.coloraxis.colorbar.x).toBeWithin(0.591, 0.01);
expect(gd.layout.coloraxis.colorbar.y).toBeWithin(0.045, 0.01);
expect(gd._fullLayout.coloraxis.colorbar.x).toBeWithin(0.591, 0.01);
expect(gd._fullLayout.coloraxis.colorbar.y).toBeWithin(0.045, 0.01);
})
.catch(failTest)
.then(done);
});

This ⬆️ one is odd. Something might be up with {editable: true} colorbar drag interactions?

it('@gl should clear graph and replot when canvas and WebGL context dimensions do not match', function(done) {
var fig = Lib.extendDeep({}, require('@mocks/splom_iris.json'));
fig.layout.showlegend = false;
function assertDims(msg, w, h) {
var canvas = gd._fullLayout._glcanvas;
expect(canvas.node().width).toBe(w, msg);
expect(canvas.node().height).toBe(h, msg);
var gl = canvas.data()[0].regl._gl;
expect(gl.drawingBufferWidth).toBe(w, msg);
expect(gl.drawingBufferHeight).toBe(h, msg);
}
var methods = ['cleanPlot', 'supplyDefaults', 'doCalcdata'];
methods.forEach(function(m) { spyOn(Plots, m).and.callThrough(); });
function assertFnCall(msg, exp) {
methods.forEach(function(m) {
expect(Plots[m]).toHaveBeenCalledTimes(exp[m], msg);
Plots[m].calls.reset();
});
}
spyOn(Lib, 'log');
Plotly.plot(gd, fig).then(function() {
assertFnCall('base', {
cleanPlot: 1, // called once from inside Plots.supplyDefaults
supplyDefaults: 1,
doCalcdata: 1
});
assertDims('base', 600, 500);
expect(Lib.log).toHaveBeenCalledTimes(0);
spyOn(gd._fullData[0]._module, 'plot').and.callThrough();
return Plotly.relayout(gd, {width: 4810, height: 3656});
})
.then(function() {
assertFnCall('after', {
cleanPlot: 3, // 2 from supplyDefaults, once in drawFramework
supplyDefaults: 2, // 1 from relayout, 1 in drawFramework
doCalcdata: 1 // once in drawFramework
});
assertDims('after', 4810, 3656);
expect(Lib.log)
.toHaveBeenCalledWith('WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug. Clearing graph and plotting again.');
})
.catch(failTest)
.then(done);
});

This ⬆️ one might means that one the most painful workaround in the library got fixed (see #2939)


@plotly/plotly_js I won't have time to fixup the jasmine tests before my vacation. This is not at all a high priority issue, but if you're interested in getting this merged, feel free to push to this branch.

package.json Outdated
@@ -43,7 +43,7 @@
"start-image_viewer": "node devtools/image_viewer/server.js",
"start": "npm run start-test_dashboard",
"baseline": "node tasks/baseline.js",
"preversion": "check-node-version --node 10 --npm 6 && npm-link-check && npm ls --prod",
"preversion": "check-node-version --node 12.13 --npm 6.12.0 && npm-link-check && npm ls --prod",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@archmoj I chose slightly more strict version checks this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now requesting for npm 6.13.x 7759c76

@etpinard
Copy link
Contributor Author

@archmoj I got the test to pass on CI and locally. Tagging as reviewable !

@archmoj
Copy link
Contributor

archmoj commented Nov 21, 2019

Thanks very much @etpinard
💃

@etpinard etpinard merged commit 9ee4aa7 into master Nov 21, 2019
@etpinard etpinard deleted the use-node12 branch November 21, 2019 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants